Fix failing integration tests (#442) by elharo · Pull Request #499 · apache/maven-resources-plugin

elharo · 2026-08-01T12:23:15Z

Disables Jenkins because it doesn't provide maven 4.0.0-rc-4 and RC5 is buggy
GitHub Actions (which runs the ITs with 4.0.0-rc-4) passes across the whole OS/JDK matrix.

Maven 4.0.0-rc-5 introduced a bug in EnhancedCompositeBeanHelper where field accessibility state was cached globally, which could cause plugin configuration injection to fail when the same configuration field is accessed multiple times or in different contexts during a build (e.g., when both 'resources' and 'testResources' goals run in the same build). This caused 4 integration test failures: - escapeInterpolation, filter-test-resources, MRESOURCES-77: build filters were not applied to test resources because the buildFilters field was not properly injected for TestResourcesMojo - MRESOURCES-131: the skip configuration did not work for testResources because TestResourcesMojo.skip was not properly injected Fixes: 1. TestResourcesMojo: Remove the private 'skip' field that shadows the parent class field (ResourcesMojo.skip), which triggered the rc-5 field accessibility caching bug. Replace with isTestSkip() method that checks: - The inherited isSkip() (handles <skip>true</skip> in plugin config) - The 'maven.test.skip' session property (handles -Dmaven.test.skip=true) 2. ResourcesMojo: Add fallback in getCombinedFiltersList() to read build filters directly from the project model when the buildFilters field was not properly injected. This makes filter loading robust against field injection failures in Maven 4 when processing class hierarchies. 3. Upgrade mavenVersion from 4.0.0-rc-4 to 4.0.0-rc-5 so the CI tests against the Maven version that exposed these issues. The Maven-side fix is in 4.0.0-rc-6 (apache/maven#11433), but these plugin-side changes ensure correct behavior with Maven 4.0.0-rc-5 as well. Closes #442

Copilot

Pull request overview

This PR addresses Maven 4.0.0-rc-5 integration test failures by making test-resources execution more resilient to Maven’s field-injection edge cases and by aligning the project’s target Maven version with the failing environment.

Changes:

Removes TestResourcesMojo’s shadowing skip field and replaces it with isTestSkip() that checks both the inherited plugin skip parameter and maven.test.skip from the session.
Adds a fallback in ResourcesMojo#getCombinedFiltersList() to read build filters directly from the project model when buildFilters injection is missing.
Bumps <mavenVersion> to 4.0.0-rc-5 in pom.xml.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/main/java/org/apache/maven/plugins/resources/TestResourcesMojo.java	Avoids `skip` field shadowing and adds session-based `maven.test.skip` handling.
src/main/java/org/apache/maven/plugins/resources/ResourcesMojo.java	Makes build filter resolution robust when `buildFilters` is not injected.
pom.xml	Updates the Maven prerequisite/target version to `4.0.0-rc-5`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+     * and the {@code maven.test.skip} property from the session.
+     *
+     * @return {@code true} if test resources should not be copied
+     * @since 3.x


elharo · 2026-08-01T12:26:21Z

-     * Its use is NOT RECOMMENDED, but quite convenient on occasion.
-     * @since 2.6
-     */
-    @Parameter(property = "maven.test.skip", defaultValue = "false")


For compatibility reasons, the parameter can't be removed here, even if it's use is a no-op

elharo · 2026-08-01T12:26:47Z

+     * @return {@code true} if test resources should not be copied
+     * @since 3.x
+     */
+    protected boolean isTestSkip() {


protected --> private

elharo · 2026-08-01T12:27:01Z

+     * and the {@code maven.test.skip} property from the session.
+     *
+     * @return {@code true} if test resources should not be copied
+     * @since 3.x


remove this line

elharo · 2026-08-01T12:29:49Z


  <properties>
-    <mavenVersion>4.0.0-rc-4</mavenVersion>
+    <mavenVersion>4.0.0-rc-5</mavenVersion>


can we jump to 4.0.0-rc-6?

RC4 and RC5 compatibility are not required

I tried to jump to 4.0.0-rc-6, but it is not yet published to Maven Central (only 4.0.0-rc-5 is available there; rc-6 is still in the Apache staging repository). The CI workflow for this repo also runs the integration tests with 4.0.0-rc-4 and cannot be changed in this PR (no workflow scope on the token), so a newer mavenVersion fails the plugin prerequisite check during the ITs. To keep CI green I reverted mavenVersion to 4.0.0-rc-4 (matching CI) and fixed the failing integration tests at the code level instead. All 27 ITs pass locally with 4.0.0-rc-4. We can bump mavenVersion + the CI workflow to rc-6 as soon as it is published and a workflow-scoped push is available.

Update: the real root cause turned out to be apache/maven#11425, fixed in 4.0.0-rc-6: the rc-5 enhanced configurator silently fails to write private fields via direct field injection (field-accessibility cache leak), leaving parameters without setters at their JVM defaults. That is exactly why escapeString, skip, buildFilters and useBuildFilters were ignored in the failing ITs.

This PR now adds setters for those parameters (setSkip, setEscapeString, setBuildFilters, setUseBuildFilters). The configurator prefers setter methods, which do not need setAccessible and therefore work on rc-4, rc-5 and rc-6 alike. All 27 ITs now pass under both 4.0.0-rc-4 and 4.0.0-rc-5.

I kept mavenVersion at 4.0.0-rc-4 because rc-6 is not yet in Maven Central (it is still in the Apache staging repository) and the repo CI workflow runs the ITs with 4.0.0-rc-4 and cannot be changed from this PR (no workflow scope). Happy to bump to rc-6 (and the CI workflow) as soon as it is published.

Correction: after further discussion I reverted the setter-based workaround. Following the same convention as maven-jar-plugin (which closed its 4.0.0-rc-5 bump waiting for 4.0.0-rc-6), this PR keeps mavenVersion on 4.0.0-rc-4 and does not try to paper over the rc-5 configurator bug (apache/maven#11425) in the plugin. All 27 ITs pass under rc-4 (verified locally and on the GitHub Actions matrix). The rc-5 failures are fixed by Maven itself in 4.0.0-rc-6; once that is published to Maven Central we can bump mavenVersion and the CI workflow (which currently pins rc-4).

@SInCE

Keep the maven.test.skip parameter for backward compatibility, make isTestSkip() private, and drop the non-specific @SInCE 3.x tag. Also revert mavenVersion to 4.0.0-rc-4 so the plugin prerequisite matches the Maven version CI uses to run the integration tests.

…442)

elharo · 2026-08-01T13:23:14Z

retest this please

elharo · 2026-08-01T17:07:29Z

I think the problem at root is that maven-resources-plugin is tripping over a bug in RC5 and Jenkins does not have RC4 installed. TBH we should drop jenkins.

elharo requested a review from Copilot August 1, 2026 12:24

Copilot started reviewing on behalf of elharo August 1, 2026 12:24 View session

Copilot AI reviewed Aug 1, 2026

View reviewed changes

Comment thread src/main/java/org/apache/maven/plugins/resources/TestResourcesMojo.java Outdated

* and the {@code maven.test.skip} property from the session.

*

* @return {@code true} if test resources should not be copied

* @since 3.x

elharo commented Aug 1, 2026

View reviewed changes

elharo changed the title ~~Fix failing integration tests with Maven 4.0.0-rc-5 (#442)~~ Fix failing integration tests (#442) Aug 1, 2026

Fix integration tests by using setter injection for mojo parameters (#…

d55b841

…442)

elharo marked this pull request as draft August 1, 2026 13:24

elharo added 7 commits August 1, 2026 13:28

Remove setter injection workaround; stay on Maven 4.0.0-rc-4 (#442)

35b95fc

pin 4.0.0-rc-4

9175610

pin 4.0.0-rc-4

a7d52d0

maven-invoker-plugin

0567c43

requiredMavenVersion

5b383b7

plugin

cb5c9d8

fix

19b6436

Jenkins is borked. Turn it off.

56d60d6

elharo marked this pull request as ready for review August 2, 2026 11:00

elharo requested a review from cstamas August 2, 2026 11:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix failing integration tests (#442) - #499

Fix failing integration tests (#442)#499
elharo wants to merge 11 commits into
masterfrom
fix/issue-442-failing-integration-tests-rc5

elharo commented Aug 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

elharo Aug 1, 2026

Uh oh!

elharo Aug 1, 2026

Uh oh!

elharo Aug 1, 2026

Uh oh!

elharo Aug 1, 2026

Uh oh!

elharo Aug 1, 2026

Uh oh!

elharo Aug 1, 2026

Uh oh!

elharo Aug 1, 2026

Uh oh!

elharo commented Aug 1, 2026

Uh oh!

elharo commented Aug 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

elharo commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

elharo Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

elharo Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

elharo Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

elharo Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

elharo Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

elharo Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

elharo Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

elharo commented Aug 1, 2026

Uh oh!

elharo commented Aug 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

elharo commented Aug 1, 2026 •

edited

Loading